There are inconsistencies with how I am testing the limits of QC and NC in mp_thompson code. And why are we needing to make droplet number in radiation code? Shouldn't this only be done in one place (MP code) and the resulting number just be given to radiation schemes?

I don't like all the duplication of code for fear that something goes awry/different in one than another place. If we need to make another isolation of ensuring species number and mass jointly make sense, then let's do it once and never more than once. But radiation code seems weird to me calling droplet or ice number fixes.

@joeolson42 @tanyasmirnova @hannahcbarnes was the reason for this the logic in the sgscloud_radpre and sgscloud_radpost schemes, i.e. the modification of qc and qi before radiation with boundary layer clouds from MYNN PBL in sgscloud_radpre, and then restoring the old values in ``sgscloud_radpost`?

We have to add the call to make_DropletNumber here only for the case when there are subgrid clouds created in PBL and convective schemes. They create sub-grid scale mixing ratios, but no NCs. After that the calc_effectRad is called using consistent mixing rations and nc_mp and ni_mp. Effective radii are needed for the radiation. After call to radiation, the sub-grid clouds are subtracted to restore the resolved mixing ratios from Thompson MP.

To save some memory, shouldn't we be using scalar (singular) values inside the loops not arrays? Unless we are saving them for later usage, in which case I guess it's necessary.

I think the loops in lines 149-166 and in lines 169-178 can be combined, which would allow us to make rho and orho scalars.

I added here just one line at request from Dustin Swales.

Note that this line is also incorrect, because it multiplies arrays p_lay(1:nCol,1:nLev) and t_lay(1:nCol,1:nLev), i.e. entire 2-dim arrays, with a scalar qv_mp(iCol,iLay) and assigns it to a scalar rho(iCol,iLay). This leads to compile errors in debug mode. I am going to fix this in the PR I am preparing for @tanyasmirnova.

Of course! A copy/paste problem Thank you for fixing this.

Same comment as before. This becomes a 3rd time for possible differences in code to exist. I don't see it as necessary.

Is this possibly because timestep#1 itself calls radiation before microphysics? If so, we should resolve this problem permanently like it can be done in WRF using module_initialize_real.F to ensure no mass can exist without a corresponding number for any MP scheme for any species in 2 moments. It would be far more general and safe that way.

I don't recall the reasoning, but it shouldn't be the timestep, because mp_thompson_init runs before any of the _run routines. @tanyasmirnova @joeolson42 @hannahcbarnes made this cloud-radiation interaction changes originally. I hope they can comment.

To address your other comment about 3rd time. If it isn't necessary for rrtmgp, then it isn't necessary for rrtmg and will disappear in both. Let's hear what the GSL developers have to say.

I don't know the history why this call to make_dropletnumber() is in here.
But when using subgrid scale clouds, the cloud-fields are adjusted just prior to calling this routine in https://github.com/NCAR/ccpp-physics/blob/master/physics/module_SGSCloud_RadPre.F90.

My assumption was that the #-concentration needed to be updated consistently before calling radiation?
Hence why it is the same in RRTMGP as RRTMG.

I commented earlier on make_DropletNumber. We have to do it every time step for subgrid clouds.

@dustinswales Your assumption is correct.

The rrtmg_pre code prepares effective radii for the calls to progcld* for all microphysics schemes. Therefore it is logical to call calc_effecRad for THompson MP in this subroutine. RRTMGP has a separate subroutine for Thompson MP: GFS_rrtmgp_thompsonmp_pre.F90.

Hmm, you are right.

I strongly urge the group to consider coordinating a single place where we treat creation of water droplet number or ice crystal number for sub-grid-scale clouds in a unified single location, not among one or more variants of radiation physics. I am already handling explicit/grid-scale clouds within my MP scheme in a uniform way. We should do likewise for SGS clouds. And, in so doing, we can also permit potential other SGS cloud schemes to do likewise. In my view, this should be really simple to do in the file Dustin mentioned (module_SGSCloud_RadPre.F90). Could you also look over my code mods in PR#567 and note that I have included specific size constraints as parameter constants at top of MP-thompson that could be linked via USE ..., ONLY .... statements? And, in a related note, my changes to the radiative effective radii also removed the internal size constraints such that after calling the size calculation, the MIN/MAX statements are now quite important. This permits the actual calculation within the subroutine to go beyond the lower/upper limits in contrast to the old behavior.

@gthompsnJCSDA
In the RRTMGP enabled physics we collect all of the MP specific information to a single module (this one in the case of Thompson MP), a simple MP-2-radiation interface. There are other modules that do the same thing for different MP/radiation couplings (e.g GFS_rrtmgp_gfdlmp_pre.F90).

Conceptually, the SGS clouds are not related to this step in the process. The SGS clouds come in from other physical parameterizations (e.g. BL and convection) and the role of the sgsCloud_radpre routine is to couple these other MP to the cloud fields, which needs to be done before we couple the MP to the radiation.

I see no problem with importing min/max's if there needs to be bounding on the effective radii. I do exactly that for the RRTMGP particle sizes.

Same comment about memory footprint and possibly using a local scalar variable not arrays.

Yes, in this place certainly possible. One could also define another scalar orho = 1/rho and do only one division instead of two (754, 763).

This should already be a part of PR#567

If it is, I'll be giving a merge conflict to resolve, so no problem.